feat: allow using SafeNoSync for MDBX #18945
Conversation
eabz
requested review from
Rjected,
gakonst,
joshieDo,
klkvr,
mattsse,
rakita and
shekhirin
as code owners
eabz
changed the title
mattsse
left a comment
mattsse
left a comment
There was a problem hiding this comment.
supportive but I'd like to change the setup for this a bit so that we don't always need to provide the syncmode
unclear to me why some changes are now using SafeNoSync
this change should not alter existing logic
|
Hey @mattsse, thanks for the feedback! I’m really enthusiastic about making my first contribution to the reth repository. Additionally, I removed all the modifications I had made to the test functions. I believe this approach aligns much better with the current codebase. I’m still getting familiar with how things work in Let me know if there is something else I need to change. |
|
Changed the parser to be easier by creating a Let me know if there are any other modifications required. |
shekhirin
left a comment
shekhirin
left a comment
There was a problem hiding this comment.
Nice, this is useful. LGTM!
| /// Controls how aggressively the database synchronizes data to disk. | ||
| #[arg( | ||
| long = "db.sync-mode", | ||
| value_parser = value_parser!(SyncMode), |
There was a problem hiding this comment.
unsure if we even need this now that syncmode is fromstr
Co-authored-by: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com>
4 participants
This PR introduces a flag to enable to users to initialize the database with
SafeNoSyncmode.Fixes: #18565
Introduces:
sync_modeto theDatabaseArgumentsandDatabaseArgs.TypedParserto make sure onlyDurableandSafeNoSyncoptions are used.DatabaseArguments::newfunction.DatabaseArguments::newis used to use theSafeNoSyncmode.Durable.DurableCan you point me if the documentation update is necessary?